-
-
Notifications
You must be signed in to change notification settings - Fork 2.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Enable buffering both reads and writes #1558
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Besides @hawkw 's comments this LGTM 👍
The clippy error seems to be in some unrelated code? |
Merging master should fix the clippy failure (#1569). |
tokio-io/src/io/async_write_ext.rs
Outdated
/// small reads and writes are batched to the underlying stream. | ||
/// | ||
/// See [`BufStream`] for details. | ||
fn buffered_duplex(self) -> BufStream<Self> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the past, fns that require both AsyncRead + AsyncWrite
have been defined on the read half.
From an API point of view, I think it is common to have types that implement both Also, what are the pros / cons to adding |
@carllerche Originally there was no method on the extension traits. @hawkw and @LucioFranco requested it be added in #1558 (comment). I'd be happy to remove them again. Perhaps it makes sense to only expose the duplex version in the ext trait, and not the one-way versions? |
Personally, my vote is to leave all the constructor fns as This question comes up repeatedly, so we probably should figure out some rules of thumb so we can be consistent across the misc types. |
Done. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
BufWriter
andBufReader
did not previously forward the "opposite" trait (AsyncRead
forBufWriter
andAsyncWrite
forBufReader
). This meant that there was no way to have both directions buffered at once. This patch fixes that, and introduces a convenience type + constructor for this double-wrapped construct.